[media-library] Add missing image loader for MediaLibrary in bare workflow #8304
Conversation
|
Native Component List for this branch is ready |
🛠 Suggested fixes:📋 Missing changelogApply suggested changes:diff --git a/packages/expo-media-library/CHANGELOG.md b/packages/expo-media-library/CHANGELOG.md
index bc2ee3c3c9..9f363b2998 100644
--- a/packages/expo-media-library/CHANGELOG.md
+++ b/packages/expo-media-library/CHANGELOG.md
@@ -8,5 +8,6 @@
### 🐛 Bug fixes
+- Add missing image loader for MediaLibrary in bare workflow. ([#8304](https://github.com/expo/expo/pull/8304) by [@tsapeta](https://github.com/tsapeta))
- Fixed `MediaLibrary` not compiling with the `use_frameworks!` option in the bare React Native application. ([#7861](https://github.com/expo/expo/pull/7861) by [@Ashoat](https://github.com/Ashoat))
- Flip dimensions based on media rotation data on Android to match `<Image>` and `<Video>` as well as iOS behavior. ([#7980](https://github.com/expo/expo/pull/7980) by [@Ashoat](https://github.com/Ashoat))or merge this pull request: #8306 |
packages/expo-media-library/ios/EXMediaLibrary/EXMediaLibraryImageLoader.h
Outdated
Show resolved
Hide resolved
packages/expo-media-library/ios/EXMediaLibrary/EXMediaLibraryImageLoader.m
Show resolved
Hide resolved
| @@ -19,4 +19,5 @@ Pod::Spec.new do |s| | |||
| s.dependency 'UMCore' | |||
| s.dependency 'UMPermissionsInterface' | |||
| s.dependency 'UMFileSystemInterface' | |||
| s.dependency 'React' | |||
There was a problem hiding this comment.
Pity that we need to add this line, but this shouldn't break anything!
|
|
||
| // Note: PhotoKit defaults to a deliveryMode of PHImageRequestOptionsDeliveryModeOpportunistic | ||
| // which means it may call back multiple times - we probably don't want that | ||
| imageOptions.deliveryMode = PHImageRequestOptionsDeliveryModeHighQualityFormat; |
There was a problem hiding this comment.
Does this mean that if a user attempts to upload a ph:// URI directly using fetch, the image might get upscaled (increase in size)? In stock React Native there’s the ph-upload:// to be used in uploads to avoid the upscaling issue
There was a problem hiding this comment.
Yeah, I think you may be right 🤔 My goal was to make it possible to render images with such urls and generally this is the same as photo library image loader from RN: RCTPhotoLibraryImageLoader.mm.
However, looks like it doesn't handle ph-upload:// urls and for that case we would need to copy RCTAssetsLibraryRequestHandler.mm as well. They both provide similar functionality and they both fix an issue with rendering, so I thought we don't need both of them at once. Also looks like stock React Native doesn't include them by default since this is the old code of CameraRoll from before it's been moved to separate repo.
Is this thing important to you? Would you be able to open similar PR but copying request handler functionality?
There was a problem hiding this comment.
It’s not important to me personally, I just figured I’d mention it :)
There was a problem hiding this comment.
Ok, cool, I'll try to fix this anyway before the release 😉 Thanks for pointing this out! 🙇♂️
Why
Fixes #7826
How
EXMediaLibraryImageLoadermodule that implementsRCTImageURLLoaderso it will be used to load images withassets-library://andph://schemes.Test Plan
Tested MediaLibrary examples in both bare and manage workflows. Photos are rendering correctly now.